Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(#8074): add filter by parent to db-object widget #8759

Merged
merged 16 commits into from
Jan 10, 2024
Merged

feat(#8074): add filter by parent to db-object widget #8759

merged 16 commits into from
Jan 10, 2024

Conversation

latin-panda
Copy link
Contributor

@latin-panda latin-panda commented Dec 14, 2023 •
edited
Loading

Description

  • This feature is applied when using the appearance:descendant-of-current-contact
  • The scope remains for forms that are opened in the contact tab; it takes the contact ID from the URL.
  • It uses the CouchDB view contact_by_parent in combination with the contact type.

Usage

type name label:en appearance
string a-person select a person select-contact type-person descendant-of-current-contact

#8074

CHT Docs PR

Code review checklist

  • Readable: Concise, well named, follows the style guide, documented if necessary.
  • Documented: Configuration and user documentation on cht-docs
  • Tested: Unit and/or e2e where appropriate
  • Internationalised: All user facing text
  • Backwards compatible: Works with existing data and configuration or includes a migration. Any breaking changes documented in the release notes.

Compose URLs

If Build CI hasn't passed, these may 404:

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.

@latin-panda
Copy link
Contributor Author

@jkuester I still need to complete the e2e, but I wanted your feedback on this work. I tried not to make big changes in the code and to keep it simple for app builders.

@latin-panda
Copy link
Contributor Author

@Benmuiruri @tatilepizs, welcome to review as well.

@tatilepizs, do you think waiting for Enketo's e2e refactor is better? I'm still trying to figure out how to check the dropdown options.

@tatilepizs
Copy link
Contributor

Hey,

do you think waiting for Enketo's e2e refactor is better?

Depending on the urgency of this feature. I sent the PR to review yesterday, and I expect feedback until next year because a lot of us will be on vacation these last weeks of the year. So I don't think that the PR will be merged soon.

I'm still trying to figure out how to check the dropdown options.

I didn't add a common option to validate dropdowns because I think that only one test that we have right now contains the dropdown widget.
This is the tests (enketo-widgets.wdio-spec.js) and this is the page object file (enketo-widgets.wdio.page.js) that evaluates the widget.
If you think that it will be a good idea to include a dropdown selector in the common ones please let me know and we can do it 😄

Thank you @latin-panda

@latin-panda latin-panda linked an issue Dec 15, 2023 that may be closed by this pull request
Support filtering contact search in forms by descendants of the current contact #8074
Closed
jkuester
jkuester requested changes Dec 15, 2023
View reviewed changes
Copy link
Contributor

@jkuester jkuester left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks great! 🤩 Very clean and well tested!

One concern I have is around the naming of with-same-parent. To me, that sounds like it will be filtering the searchable contacts to be limited to siblings of the contact in context (the ones that have the same parent as the current contact). However, with this implementation, it is actually limiting the searchable contacts to children of the contact in context (and all their descendants).

An appearance name more like descendant-of-current-contact would make more sense to me (not a huge fan of that name either, but it seems more accurate and I have not been able to think of a better one)....

Also, just a reminder that we definitely want to update the docs to describe this functionality!

shared-libs/search/src/generate-search-requests.js Outdated Show resolved Hide resolved
shared-libs/search/src/generate-search-requests.js Outdated Show resolved Hide resolved
webapp/src/ts/services/select2-search.service.ts Outdated Show resolved Hide resolved
@latin-panda latin-panda mentioned this pull request Dec 18, 2023
Merged
@latin-panda
Copy link
Contributor Author

@michaelkohn What do you think about Josh's idea of using descendant-of-current-contact instead of with-same-parent?

IMO, it's long but descriptive. I prefer things to be descriptive :)

latin-panda
latin-panda commented Dec 18, 2023
View reviewed changes
tests/page-objects/default/enketo/generic-form.wdio.page.js
Comment on lines +117 to +118
const firstElement = $('.select2-results__options > li');
await (await firstElement).waitForClickable();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was struggling with this e2e being flaky, this seems to work.

@latin-panda latin-panda marked this pull request as ready for review December 18, 2023 12:14
tatilepizs
tatilepizs requested changes Dec 18, 2023
View reviewed changes
Copy link
Contributor

@tatilepizs tatilepizs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @latin-panda for adding the e2e test.
I just left a comment in place.

tests/e2e/default/enketo/db-object-widget.wdio-spec.js Show resolved Hide resolved
tests/e2e/default/enketo/forms/db-object-widget.xml Show resolved Hide resolved
jkuester
jkuester approved these changes Jan 4, 2024
View reviewed changes
Copy link
Contributor

@jkuester jkuester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Benmuiruri
Benmuiruri approved these changes Jan 5, 2024
View reviewed changes
Copy link
Contributor

@Benmuiruri Benmuiruri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work!

@latin-panda
Copy link
Contributor Author

@tatilepizs When you have time, can you please review this again?

tatilepizs
tatilepizs approved these changes Jan 9, 2024
View reviewed changes
Copy link
Contributor

@tatilepizs tatilepizs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good @latin-panda, thank you!! 🌟

@latin-panda latin-panda merged commit 4547885 into master Jan 10, 2024
37 checks passed
@latin-panda latin-panda deleted the 8074-new-dbobject-filter branch January 10, 2024 04:17
latin-panda added a commit to medic/cht-docs that referenced this pull request Jan 10, 2024
@latin-panda
chore: documentation of new filter in db-object widget (#1254)
46158b6 
Documentation for the new filter in db-object widget
PR in CHT-Core: medic/cht-core#8759
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorerod lorerod lorerod left review comments

@tatilepizs tatilepizs tatilepizs approved these changes

@jkuester jkuester jkuester approved these changes

@Benmuiruri Benmuiruri Benmuiruri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support filtering contact search in forms by descendants of the current contact
5 participants
@latin-panda @tatilepizs @Benmuiruri @jkuester @lorerod